Skip to content

Conversation

@JeffersonBledsoe
Copy link
Member

@JeffersonBledsoe JeffersonBledsoe commented Mar 31, 2025

Currently, if you anonymously get data from a field which points to an object which the requirements for an object with a PrimaryField (e.g. a block with a url field which points to a dexterity file object), you are always given the @@download version of the URL. This isn't always desirable as you may have content on the 'display' version of the view.

This PR changes this view to expand into information about the primary field target so that the consumer of the REST API response (e.g. Volto) can make the decision on whether to open the content object or download it. Currently this is behind an environment variable to test the behaviour.


📚 Documentation preview 📚: https://plonerestapi--1903.org.readthedocs.build/

@mister-roboto
Copy link

@JeffersonBledsoe thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

@JeffersonBledsoe JeffersonBledsoe changed the title WIP: Option to transform primary field target to provide information about the target rather than just it's URL Option to transform primary field target to provide information about the target rather than just it's URL Mar 31, 2025
@JeffersonBledsoe JeffersonBledsoe marked this pull request as draft July 3, 2025 09:31
JeffersonBledsoe added a commit to pretagov/plone.restapi that referenced this pull request Sep 30, 2025
…de information about the target rather than just it's URL plone#1903
JeffersonBledsoe added a commit to pretagov/plone.restapi that referenced this pull request Sep 30, 2025
@JeffersonBledsoe
Copy link
Member Author

@davisagli I've been having some issues trying to complete this and wanted to check it made sense as it would be a breaking change without the environment variable. The functionality mostly works well, but as you can see from the latest test runs, the @id field is also being expanded, when this should really still be just a string to the URL of the referenced object. I see the following ways of moving forward with this PR:

  • Find a way to check whether we're converting an @id field and ensure we don't run the expansion then (unsure how to do this currently)
  • Change approach completely. We could have an additional field of "expanded objects" where the URLs that get created from the resolveuid expansions are the keys and the values contain information from the objects.
  • I've misunderstood the primary file target field along the way and there's another way to get information about a file field without making another request.

@davisagli
Copy link
Member

@JeffersonBledsoe I think the idea is sound in general; the API is not providing enough information here.

In terms of backwards-compatibility, I'd suggest following this path:

  1. add it on main but only when the environment variable is present
  2. test volto with the environment variable present and make any necessary updates to handle the new structure
  3. make a future major release of plone.restapi that has it enabled by default

About the extra expansion of the @id, I think the problem is that

class ResolveUIDSerializerBase:
is really expecting resolve_uid to return a str, but now you've made a change that makes it return a dict in this case.

Possible solutions:

  1. Update ResolveUIDSerializerBase to handle a return value that is a dict, and merge it into the existing dict (but maybe we'll run into other things that are expecting resolve_uid to always return a str)
  2. Move your code to do extra expansion into ResolveUIDSerializerBase: make it check if the resolved str URL includes @@Download, and if so add the other fields. This would be a bit similar to the special case it already has for image_scales.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants